Skip to content
This repository was archived by the owner on Mar 13, 2025. It is now read-only.

issue 46 fix #561

Closed
wants to merge 3 commits into from
Closed

issue 46 fix #561

wants to merge 3 commits into from

Conversation

PrakashDurlabhji
Copy link
Contributor

@PrakashDurlabhji PrakashDurlabhji commented Jul 23, 2020

#46

Copy link
Collaborator

@callmekatootie callmekatootie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Issue is not resolved. The group label in the edit profile modal continues to be exceeding the width it is in
  • Your changes also have side effect - the padding on the pill appears to have increased and not following the original one

@PrakashDurlabhji
Copy link
Contributor Author

@callmekatootie yes i increased a bit padding, i will reset to original one. and i will paste a verification screenshot for the fix now. It was visible and working in edit profile.

@PrakashDurlabhji
Copy link
Contributor Author

@callmekatootie screenshot
Capture

@callmekatootie
Copy link
Collaborator

@PrakashDurlabhji The changes are incorrect. You have used word break but you were expected to only reduce the width of the pill. Please see this: #46 (comment)

I am afraid that if this does not get resolved in the next 2 hours, I will have to open this for others to pickup

@PrakashDurlabhji
Copy link
Contributor Author

@callmekatootie in 1 hour i will complete

@PrakashDurlabhji
Copy link
Contributor Author

@callmekatootie can you please check again? I updated

@callmekatootie
Copy link
Collaborator

@PrakashDurlabhji I am afraid the fix is not acceptable - you have shrunk the pill size but that is not the right way to go about it - the pill size should take up the container size it is in and have a max width... shrinking the max width would not be a good solution

@PrakashDurlabhji
Copy link
Contributor Author

@callmekatootie i can solve this but break-word is required if we want to show full text. or i need to show ellipsis? is ellipsis ok?

@callmekatootie
Copy link
Collaborator

Right now, ellipsis is being showed.

The fix expected is that the pill should have a max width (which it does currently and it is fine) AND if the container in which the pill exists is having a shorter width, the pill should not exceed the container's width

@PrakashDurlabhji
Copy link
Contributor Author

@callmekatootie yes understood, now you will be satisfied with my approach. updating it

@PrakashDurlabhji
Copy link
Contributor Author

@callmekatootie will this be ok? as i cannot control child pills width also you said i cannot alter pill css,so i have to apply changes to parent div

Capture

@callmekatootie
Copy link
Collaborator

callmekatootie commented Jul 24, 2020

No, I am afraid that is not acceptable.

You can alter the Pill CSS - but not by playing with it's max width property...

@callmekatootie callmekatootie deleted the issue_46 branch July 24, 2020 15:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants